-
Notifications
You must be signed in to change notification settings - Fork 2
Solution (JavaScript) #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you change your implementation to support a radix other than 10?
{input: [4, 9, 9, 9], output: [5, 0, 0, 0]}, | ||
{input: [9, 9, 9, 9], output: [0, 0, 0, 0]}, | ||
] | ||
itParam("should increment array 'number'", cases, function({input, output}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent use of itParam
.
src/index.js
Outdated
odometerReverse = function(array) { | ||
let minusOne = true | ||
|
||
for (let i = array.length; i--; i > -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting... this clearly works, but doesn't look like a normal for
loop. Can you explain why it works as-is? How else might you write this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, very interesting indeed... I believe what is happening is when the loop runs, the second statement, i--
, is decrementing. Normally this would happen in the third statement but that always returning true
. However the "should loop continue" statement is getting to 0 and terminating the loop. I believe this is demonstrated by this repl https://repl.it/@dustinyschild/CrushingStripedWorker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This loop should be rewritten as follows:
for (let i = array.length - 1; i === 0; i--)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope... But this is correct. Should have ran tests first!
for (let i = array.length - 1; i > -1; i--)
Could you provide more information as to how a user would be representing numbers with bases greater than 10? For example, would hexadecimal digits be represented as |
Let's assume you'll receive either all numbers or all strings. How would make sure the output is in the same format as the input? |
support for returning input type for output values
@dahlbyk I pushed a change that will allow for this function to return the same type that was provided, I ran into an edge case where the provided number could be all integers but the return value could not return all integers. I decided it would force all values to strings in this special case. Also this is the point I would start breaking this down into smaller functions. |
This is my solution to the odometer code challenge. It includes my implementation as well as parameterized tests that I used to write this code.
To run tests:
npm install
npm test